Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix issue (GH6290) in new DialogDisplayerImpl behaviour #6294

Merged
merged 1 commit into from
Aug 4, 2023
Merged

Fix issue (GH6290) in new DialogDisplayerImpl behaviour #6294

merged 1 commit into from
Aug 4, 2023

Conversation

neilcsmith-net
Copy link
Member

Attempt to fix issue #6290 in new DialogDisplayerImpl behaviour introduced in #5989 and #6216.

Possible alternative to reverting in #6291

@neilcsmith-net neilcsmith-net added do not merge Don't merge this PR, it is not ready or just demonstration purposes. Platform [ci] enable platform tests (platform/*) ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Aug 3, 2023
@neilcsmith-net neilcsmith-net added this to the NB19 milestone Aug 3, 2023
@neilcsmith-net neilcsmith-net mentioned this pull request Aug 3, 2023
Closed
@neilcsmith-net
Copy link
Member Author

Force pushed (again) because of failing test. Actually realised the test had been updated as part of #5989 and the update here behaves like the previous NetBeans code did. Putting back the old test line.

@neilcsmith-net neilcsmith-net mentioned this pull request Aug 3, 2023
Closed
errael
errael reviewed Aug 3, 2023
View reviewed changes
Copy link
Contributor

@errael errael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't understand the whole leaf thing, but since I haven't tried I don't feel to bad. This certainly seems to avoid parenting to a Window. I'm good with it (except for the initBound()).

Took a news break while reviewing, came across https://www.cnn.com/videos/us/2023/08/02/boston-police-officer-falls-down-slide-cprog-mh-orig.cnn which somehow resonated.

@mbien
Copy link
Member

mbien commented Aug 3, 2023 •
edited
Loading

tested the last dev build of this PR and it seems to be working fine - without exceptions in the log.

I did also notice that the save/discard dialog which pops up when #4792 is used is now centered on the main window, which is better since it used the popup as parent in NB 18 believe.

@neilcsmith-net neilcsmith-net removed the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Aug 4, 2023
@neilcsmith-net
Copy link
Member Author

Thanks @mbien Also confirmation from the bug reporter on the dev build. Let's merge and test in rc4.

@neilcsmith-net neilcsmith-net merged commit 5d6c0c0 into apache:delivery Aug 4, 2023
@neilcsmith-net neilcsmith-net mentioned this pull request Sep 7, 2023
Merged
@neilcsmith-net neilcsmith-net deleted the gh6290 branch January 16, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@errael errael errael left review comments

@mbien mbien Awaiting requested review from mbien

@ebarboni ebarboni Awaiting requested review from ebarboni

@MartinBalin MartinBalin Awaiting requested review from MartinBalin

@sdedic sdedic Awaiting requested review from sdedic

Assignees
No one assigned
Labels
ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Platform [ci] enable platform tests (platform/*)
Projects
None yet
Milestone
NB19
Development

Successfully merging this pull request may close these issues.
3 participants
@neilcsmith-net @mbien @errael